Skip to content

chore(auth-server): minor code cleanup and fixes#19940

Open
sreecharan-desu wants to merge 2 commits intomozilla:mainfrom
sreecharan-desu:fix/auth-server-cleanups
Open

chore(auth-server): minor code cleanup and fixes#19940
sreecharan-desu wants to merge 2 commits intomozilla:mainfrom
sreecharan-desu:fix/auth-server-cleanups

Conversation

@sreecharan-desu
Copy link
Copy Markdown
Contributor

@sreecharan-desu sreecharan-desu commented Jan 25, 2026

This PR introduces a few targeted cleanups and improvements to the fxa-auth-server package:

  • Fix Typo: Corrected 'seperately' to 'separately' in the OAuth API test suite.
  • Unhardcode Session Token Expiry: Updated the SessionToken model to pull its expiry from the auth-server configuration instead of using a hardcoded static value.
  • Added Regression Tests: Included unit tests for the session token expiry filtering logic to ensure correctness.

These changes address the maintainer's feedback regarding targeted PRs and unhardcoding configuration values, while maintaining existing functionality.

@sreecharan-desu sreecharan-desu requested a review from a team as a code owner January 25, 2026 14:05
@github-actions
Copy link
Copy Markdown
Contributor

This pull request issue has been marked as stale due to inactivity. It will be closed in a week if it continues to be stale.

Comment thread packages/fxa-auth-server/lib/senders/emails/sass-compile-files.ts
Comment thread packages/fxa-auth-server/test/remote/oauth_api.in.spec.ts
Comment thread packages/fxa-auth-server/test/oauth/api.js Outdated
Comment thread packages/fxa-auth-server/test/oauth/api.js Outdated
Comment thread packages/fxa-shared/db/models/auth/session-token.ts
Copy link
Copy Markdown
Contributor

@dschom dschom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these contributions. This PR has grown stale, but you have some nice clean up here. Please see comments and feel free to refile more targeted PRs. We will try to address them in a timely manner this time around.

@sreecharan-desu sreecharan-desu force-pushed the fix/auth-server-cleanups branch from 1910ae5 to 3eb0581 Compare April 17, 2026 15:42
@sreecharan-desu
Copy link
Copy Markdown
Contributor Author

sreecharan-desu commented Apr 17, 2026

Thanks for the feedback, @dschom! I've updated the PR to address your requests:

  • Reverted Sass Improvements: Removed the changes to sass-compile-files.ts as they are no longer applicable.
  • Fixed Typo: Corrected 'seperately' to 'separately' in the contemporary test file location without any formatting noise.
  • Unhardcoded Session Token Expiry: The expiry is now dynamically set from the auth-server configuration instead of being a static member. I've also added unit tests in fxa-shared to verify the logic.
  • Consolidated Effort: I've integrated the typo fix directly here and closed the redundant PR (fix(auth-server): correct typo 'seperately' to 'separately' in tests #20399) to keep the cleanup focused in this thread.

I hope this targeted version is much easier to review. Let me know if you need any further adjustments!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants